Skip to content

Conversation

@newtork
Copy link
Contributor

@newtork newtork commented Oct 21, 2025

Context

Related

Considering a warn message, since we cannot enforce it via API design with the existing code.

Feature scope:

  • Log warning for incorrect API usage: Spring AI .entity(..) + Orchestration .withTemplateConfig(..)

Definition of Done

  • Functionality scope stated & covered
  • Tests cover the scope above
  • Error handling created / updated & covered by the tests above
  • Aligned changes with the JavaScript SDK
  • Documentation updated
  • Release notes updated

@newtork newtork added the please-review Request to review a pull-request label Oct 21, 2025
Comment on lines +85 to +88
val JSON_SCHEMA = "https://json-schema.org/draft/2020-12/schema";
if (t.text().contains(JSON_SCHEMA) && config.getTemplateConfig() != null) {
log.warn("Combination of `entity(...)` and `withTemplateConfig(...)` is not supported.");
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if testing just for https://json-schema.org/draft/2020-12/schema is good here. This might lead to false positives if a users specifies the schema version in their actual prompt (not sure how likely it is, but might happen?!)

I would thus add a bit of the formatting around it to the test string.

Suggested change
val JSON_SCHEMA = "https://json-schema.org/draft/2020-12/schema";
if (t.text().contains(JSON_SCHEMA) && config.getTemplateConfig() != null) {
log.warn("Combination of `entity(...)` and `withTemplateConfig(...)` is not supported.");
}
val JSON_SCHEMA = "\\n \\\"$schema\\\" : \\\"https://json-schema.org/draft/2020-12/schema\\\",";
if (t.text().contains(JSON_SCHEMA) && config.getTemplateConfig() != null) {
log.warn("Combination of `entity(...)` and `withTemplateConfig(...)` is not supported.");
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Likely we shouldn't put this in OrchestrationClient altogether. If we put this into code, it'd be better located in Spring AI related class(es) 🤔

@newtork newtork marked this pull request as draft October 22, 2025 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dont-merge please-review Request to review a pull-request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants